Fixing CLI usage not having enough rights in GLPI's profile system#22996
Fixing CLI usage not having enough rights in GLPI's profile system#22996C-Duv wants to merge 1 commit intoglpi-project:11.0/bugfixesfrom
Conversation
|
This breaks tests |
cedric-anne
left a comment
There was a problem hiding this comment.
IMHO, it is preferable to combine the usage of Session::callAsSystem() in the command with a Session::isRightChecksDisabled() check in the Profile::currentUserHaveMoreRightThan() method.
c897d76 to
a748323
Compare
@cedric-anne Something like that? src/Glpi/Console/User/ResetPasswordCommand.php | 3 +--
src/Profile.php | 5 ++++-
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/Glpi/Console/User/ResetPasswordCommand.php b/src/Glpi/Console/User/ResetPasswordCommand.php
index 05d211fd55..e234f5f172 100644
--- a/src/Glpi/Console/User/ResetPasswordCommand.php
+++ b/src/Glpi/Console/User/ResetPasswordCommand.php
@@ -76,8 +76,7 @@ class ResetPasswordCommand extends AbstractUserCommand
$user_input['password'] = $password;
$user_input['password2'] = $password;
-
- if ($user->update($user_input)) {
+ if (Session::callAsSystem(fn() => $user->update($user_input))) {
$output->writeln('<info>' . __('Reset password successful.') . '</info>');
return 0;
} else {
diff --git a/src/Profile.php b/src/Profile.php
index 047eed3522..cfbec75846 100644
--- a/src/Profile.php
+++ b/src/Profile.php
@@ -743,7 +743,10 @@ class Profile extends CommonDBTM implements LinkableToTilesInterface
{
global $DB;
- if (Session::isCron()) {
+ if (
+ Session::isRightChecksDisabled()
+ || Session::isCron()
+ ) {
return true;
}
if (count($IDs) === 0) { |
cedric-anne
left a comment
There was a problem hiding this comment.
Could you please validate it works as expected? According to the lint test failure, you did not test it, or you would have detect the failure.
|
|
||
|
|
||
| if ($user->update($user_input)) { | ||
| if (Session::callAsSystem(fn() => $user->update($user_input))) { |
There was a problem hiding this comment.
| if (Session::callAsSystem(fn() => $user->update($user_input))) { | |
| if (\Session::callAsSystem(fn() => $user->update($user_input))) { |
There was a problem hiding this comment.
Fix applied, it made vendor/bin/phpstan analyze happy.
(and the code still fixes #22931)
The CLI/console `user:reset_password` was failing to reset an user's password with: > An error occurred during password update > Unable to reset password, please contact your administrator It's caused by `User::currentUserHaveMoreRightThan()` and/or `Profile::currentUserHaveMoreRightThan()` not detecting the CLI context of execution. This commit makes sure `User::update()` is executed as the system to bypass permissions enforcement and checks for such bypass in `Profile::currentUserHaveMoreRightThan()`. Issue: glpi-project#22931
a748323 to
4fbd83d
Compare
Checklist before requesting a review
Description
Adds a check for CLI execution in
Profile::currentUserHaveMoreRightThan()so it can grants it rights if needed.